Improve auth token error formatting for easier copy-paste by jamesbroadhead · Pull Request #5115 · databricks/cli

jamesbroadhead · 2026-04-28T16:06:03Z

Replaces #4602 — re-opened from an upstream branch (was previously from a fork, which blocked CI from running properly). Original PR was approved by @andrewnester and @simonfaltum.

Changes

Format the auth token error message across multiple lines so the
suggested databricks auth login command is on its own line and easy
to copy-paste.

Before:

Error: cache: databricks OAuth is not configured for this host. Try logging in again with `databricks auth login --host https://example.com` before retrying. If this fails, please report this issue to the Databricks CLI maintainers at https://github.com/databricks/cli/issues/new

After:

Error: cache: databricks OAuth is not configured for this host.

To reauthenticate, run the following command:

  $ databricks auth login --host https://example.com

If this fails, please report this issue to the Databricks CLI maintainers at https://github.com/databricks/cli/issues/new

Why

When auth fails, the suggested login command was printed on the same line as the error and config details, making it difficult to select and copy. This follows the same multi-line pattern already used by RewriteAuthError for invalid refresh token errors.

Tests

Unit tests updated and passing (go test ./cmd/auth/ -run TestToken_loadToken)
Acceptance test golden file updated and passing (go test ./acceptance/... -run TestAccept/cmd/auth/token)

When auth fails, the suggested `databricks auth login` command was printed on the same line as the error and config details, making it hard to copy. Split the message across multiple lines so the command is visually distinct and easy to select. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…formatting # Conflicts: # NEXT_CHANGELOG.md # cmd/auth/token_test.go

…formatting

github-actions · 2026-04-28T16:07:47Z

Approval status: pending

`/cmd/auth/` - needs approval

Files: cmd/auth/token.go, cmd/auth/token_test.go
Suggested: @simonfaltum
Also eligible: @mihaimitrea-db, @tanmay-db, @renaudhartert-db, @hectorcast-db, @parthban-db, @Divyansh-db, @tejaskochar-db, @chrisst, @rauchy

`/cmd/root/` - needs approval

Files: cmd/root/auth.go
Suggested: @simonfaltum
Also eligible: @mihaimitrea-db, @tanmay-db, @renaudhartert-db, @hectorcast-db, @parthban-db, @Divyansh-db, @tejaskochar-db, @chrisst, @rauchy

`/libs/auth/` - needs approval

Files: libs/auth/error.go, libs/auth/error_test.go
Suggested: @simonfaltum
Also eligible: @mihaimitrea-db, @tanmay-db, @renaudhartert-db, @hectorcast-db, @parthban-db, @Divyansh-db, @tejaskochar-db, @chrisst, @rauchy

General files (require maintainer)

5 files changed
Based on git history:

@simonfaltum -- recent work in cmd/auth/, ./, libs/auth/

_{Any maintainer (@andrewnester, @anton-107, @denik, @pietern, @shreyas-goenka, @simonfaltum, @renaudhartert-db) can approve all areas.

See OWNERS for ownership rules.}

…command - The token-error format change in this branch landed only in acceptance/cmd/auth/token/output.txt; logout/stale-account-id-workspace-host and token/force-refresh-no-cache still asserted the old single-line message and fail CI. Regenerate both with the new multi-line format. - BuildLoginCommand now shell-quotes each argument before joining via libs/shellquote, so a profile or host containing spaces or shell metacharacters renders as a safe copy-paste command instead of a broken or unsafe one. BuildDescribeCommand quotes the profile too, for consistency. Co-authored-by: Isaac

helpfulError and BuildLoginCommand previously took only an OAuthArgument, which carries Host + AccountID + Profile but drops WorkspaceID. As a result, the suggested 'databricks auth login ...' for a token failure on a SPOG workspace hit either the wrong workspace or forced an unexpected interactive workspace selection. - Thread WorkspaceID through BuildLoginCommand and helpfulError. - Append --workspace-id to the rendered command when set, skipping empty values and the WorkspaceIDNone sentinel. - Plumb the workspace ID at the call sites: token.go (loadToken), root/auth.go (renderError), and the inline RewriteAuthError caller. - Add TestBuildLoginCommand_AppendsWorkspaceID covering profile path, unified host path, empty, and 'none' sentinel. - Regenerate the auth acceptance fixtures that exercise this path. Co-authored-by: Isaac

…formatting # Conflicts: # libs/auth/error.go # libs/auth/error_test.go

main removed AuthArguments.IsUnifiedHost in favor of DiscoveryURL-based routing (#5137). Update TestBuildLoginCommand_AppendsWorkspaceID's unified-host case to use a discovery URL containing /oidc/accounts/<id>/ so the test still exercises UnifiedOAuthArgument. Co-authored-by: Isaac

simonfaltum

Two small findings from review. The auth behavior mostly looks aligned with the existing code; these are the pieces I think should be cleaned up before merge.

simonfaltum · 2026-05-07T11:26:38Z


 * `databricks api` now works against unified hosts. Adds `--account` to scope a call to the account API and `--workspace-id` to override the workspace routing identifier per call. A `?o=<workspace-id>` query parameter on the path (the SPOG URL convention used by the Databricks UI) is also recognized as a per-call workspace override, so URLs pasted from the browser route correctly.

+* Improve `auth token` error formatting for easier copy-paste of login commands ([#4602](https://github.com/databricks/cli/pull/4602))


This link still points to the replaced PR (#4602). Since this PR is the one that will merge, the release note should reference #5115; otherwise the shipped changelog sends readers to the abandoned PR.

simonfaltum · 2026-05-07T11:26:38Z

 			return
 		}
-		fmt.Fprintf(b, "\n  - Re-authenticate: %s", BuildLoginCommand(ctx, "", oauthArg))
+		fmt.Fprintf(b, "\n  - Re-authenticate: %s", BuildLoginCommand(ctx, "", cfg.WorkspaceID, oauthArg))


This non-profile branch now gets the safer command construction, but the cfg.Profile != "" branch just above still formats databricks auth login --profile %s by hand. That means profile names with spaces or shell metacharacters still produce broken/unsafe copy-paste commands in EnrichAuthError. Can the profile branch use BuildLoginCommand(ctx, cfg.Profile, cfg.WorkspaceID, nil) too, or otherwise quote consistently?

simonfaltum

Review summary

I ran a multi-agent review (Isaac + Cursor) on this reopened PR. Both reviewers converged on COMMENT with one important consistency gap.

The core auth token formatting change is correct and well-tested for the surface it touched, but the new BuildLoginCommand helper isn't used everywhere it should be — one sibling render path was missed.

Important:

writeReauthSteps's AuthTypeDatabricksCli profile branch still uses raw fmt.Fprintf(... "--profile %s", ...), bypassing the new helper. This is the most common 401 path. Result: it misses both --workspace-id and shell-quoting in exactly the workflow this PR is supposed to be normalizing.

Nits:

Changelog links the superseded #4602 instead of #5115
RewriteAuthError layout still single-paragraph vs helpfulError's blank-line-padded form
BuildLoginCommand doc-comment doesn't explain why Account/Workspace OAuth cases drop workspaceID
Test gaps: no negative assertion for omission, no shell-quoting test

Question:

Why include --workspace-id on the profile path? auth login --profile foo already picks up the profile's stored workspace_id — worth a one-line note in the doc-comment to explain the intent (pin to the workspace the failing call resolved against?).

For full context I left these as inline comments below.

simonfaltum · 2026-05-07T14:16:08Z

 			return
 		}
-		fmt.Fprintf(b, "\n  - Re-authenticate: %s", BuildLoginCommand(ctx, "", oauthArg))
+		fmt.Fprintf(b, "\n  - Re-authenticate: %s", BuildLoginCommand(ctx, "", cfg.WorkspaceID, oauthArg))


Important: the sibling profile branch (line 125) wasn't updated.

This branch correctly routes through BuildLoginCommand(ctx, "", cfg.WorkspaceID, oauthArg) now. But the if cfg.Profile != "" branch above (line 125 in the new file) still does fmt.Fprintf(b, "... --profile %s", cfg.Profile), bypassing the helper.

Result: a 401 on a profile-based databricks-cli call (the most common case) renders without --workspace-id and without shell-quoting, while this branch and the new helpfulError path do both. That makes the PR's own consistency claim only half-true — and on unified/SPOG hosts the suggested command can target the wrong workspace.

Fix: route the profile branch through BuildLoginCommand(ctx, cfg.Profile, cfg.WorkspaceID, nil). Add a test using a profile with WorkspaceID set, and ideally a profile name needing shell-quoting (e.g. with a space) to cover both improvements at once.

simonfaltum · 2026-05-07T14:16:08Z


 * `databricks api` now works against unified hosts. Adds `--account` to scope a call to the account API and `--workspace-id` to override the workspace routing identifier per call. A `?o=<workspace-id>` query parameter on the path (the SPOG URL convention used by the Databricks UI) is also recognized as a per-call workspace override, so URLs pasted from the browser route correctly.

+* Improve `auth token` error formatting for easier copy-paste of login commands ([#4602](https://github.com/databricks/cli/pull/4602))


Nit: this links the superseded #4602, but this branch will merge as #5115. Worth pointing release-note readers at the actual merged PR.

simonfaltum · 2026-05-07T14:16:08Z

 		msg := `A new access token could not be retrieved because the refresh token is invalid. To reauthenticate, run the following command:
-  $ ` + BuildLoginCommand(ctx, profile, oauthArgument)
+  $ ` + BuildLoginCommand(ctx, profile, workspaceId, oauthArgument)


Nit: layout inconsistency.

RewriteAuthError keeps the legacy ...command:\n $ <cmd> shape, while the new helpfulError in cmd/auth/token.go uses a blank-line-padded layout (...command:\n\n $ <cmd>\n\n...). The PR description says the new format "matches the existing RewriteAuthError pattern," but they now diverge.

Compare acceptance/cmd/auth/token/force-refresh-invalid-refresh-token/output.txt (no blank lines around the command) with force-refresh-no-cache/output.txt (blank lines around the command).

Fix: unify on the new padded form here too — that's the actual goal of the PR. The acceptance test golden file would need a small update.

simonfaltum · 2026-05-07T14:16:08Z

+// BuildLoginCommand builds the login command for the given OAuth argument or
+// profile. Each argument is shell-quoted so the rendered command is safe to
+// copy-paste even when host, profile, or account-id values contain spaces or
+// shell metacharacters.
+//
+// workspaceID, when non-empty and not the WorkspaceIDNone sentinel, is
+// emitted as --workspace-id for unified hosts so the suggested reauth
+// targets the same workspace the failing call resolved against (the
+// information the OAuthArgument otherwise drops).


Nit: the doc-comment explains the unified-host case for --workspace-id but not the AccountOAuthArgument/WorkspaceOAuthArgument cases below, which intentionally drop workspaceID. This is correct (account args target the account API; workspace args identify a workspace by host already), but the asymmetry isn't called out and is easy to break in a future change.

Fix: add one sentence — "For AccountOAuthArgument and WorkspaceOAuthArgument, workspaceID is intentionally ignored since those argument types either target the account API or already identify a workspace by host."

simonfaltum · 2026-05-07T14:16:08Z

+func TestBuildLoginCommand_AppendsWorkspaceID(t *testing.T) {
+	ctx := t.Context()
+
+	t.Run("profile path emits --workspace-id when set", func(t *testing.T) {
+		cmd := BuildLoginCommand(ctx, "dev", "12345", nil)
+		assert.Equal(t, "databricks auth login --profile dev --workspace-id 12345", cmd)
+	})
+
+	t.Run("profile path omits --workspace-id when empty", func(t *testing.T) {
+		cmd := BuildLoginCommand(ctx, "dev", "", nil)
+		assert.Equal(t, "databricks auth login --profile dev", cmd)
+	})
+
+	t.Run("profile path omits --workspace-id for the 'none' sentinel", func(t *testing.T) {
+		cmd := BuildLoginCommand(ctx, "dev", WorkspaceIDNone, nil)
+		assert.Equal(t, "databricks auth login --profile dev", cmd)
+	})
+
+	t.Run("unified host path emits --workspace-id when set", func(t *testing.T) {
+		oauthArg, err := AuthArguments{
+			Host:         "https://unified.cloud.databricks.com",
+			AccountID:    "acc-123",
+			DiscoveryURL: "https://unified.cloud.databricks.com/oidc/accounts/acc-123/.well-known/oauth-authorization-server",
+		}.ToOAuthArgument()
+		require.NoError(t, err)
+
+		cmd := BuildLoginCommand(ctx, "", "ws-456", oauthArg)
+		assert.Contains(t, cmd, "--account-id acc-123")
+		assert.Contains(t, cmd, "--workspace-id ws-456")
+	})
+}


Nit: two coverage gaps in TestBuildLoginCommand_AppendsWorkspaceID:

No negative assertion that AccountOAuthArgument/WorkspaceOAuthArgument paths omit --workspace-id even when one is provided. The production code does this intentionally; a regression test would lock that in.

No shell-quoting test. All the existing cases use safe profile names (dev), so shellquote.BashArg is effectively untested. A profile name like "weird name" would cover the round-trip through the quoter.

Fix: two short table cases — one with WorkspaceOAuthArgument (e.g. classic https://adb-123.azuredatabricks.net) asserting --workspace-id is absent; one with profile = "weird name" asserting the output contains --profile 'weird name'. Both are one-liners.

simonfaltum · 2026-05-07T14:16:08Z

 	allArgs = append(allArgs, u2m.WithOAuthArgument(oauthArgument))
 	persistentAuth, err := u2m.NewPersistentAuth(ctx, allArgs...)
 	if err != nil {
-		helpMsg := helpfulError(ctx, args.profileName, oauthArgument)
-		return nil, fmt.Errorf("%w. %s", err, helpMsg)
+		helpMsg := helpfulError(ctx, args.profileName, args.authArguments.WorkspaceID, oauthArgument)


Question: why include --workspace-id on the profile path?

When the user invokes databricks auth token --profile foo and we suggest databricks auth login --profile foo --workspace-id <id>, the --workspace-id is technically redundant — auth login --profile foo will already pick up the workspace_id stored in the profile.

Including it is meaningful only if (a) the profile's persisted workspace_id has drifted from the failing call's workspace_id, or (b) the user typed --workspace-id on the failing call and we want to preserve that intent. Both are plausible, but the doc-comment on BuildLoginCommand only motivates the unified-host case.

Not asking for a code change — just want to confirm this was deliberate. If yes, a one-line addendum to the doc-comment (e.g. "…also re-emitted on the profile path so the suggested reauth pins the same workspace_id the failing call resolved against") would close the loop.

jamesbroadhead and others added 5 commits February 25, 2026 21:43

gci autocommit

368593e

Merge branch 'main' into fix-auth-login-error-formatting

412952e

Merge remote-tracking branch 'origin/main' into fix-auth-login-error-…

8a47c90

…formatting # Conflicts: # NEXT_CHANGELOG.md # cmd/auth/token_test.go

Merge remote-tracking branch 'origin/main' into fix-auth-login-error-…

3e1ef3f

…formatting

jamesbroadhead mentioned this pull request Apr 28, 2026

Improve auth token error formatting for easier copy-paste #4602

Closed

jamesbroadhead temporarily deployed to test-trigger-is April 28, 2026 16:06 — with GitHub Actions Inactive

jamesbroadhead requested review from andrewnester and simonfaltum April 28, 2026 16:07

jamesbroadhead temporarily deployed to test-trigger-is April 28, 2026 16:43 — with GitHub Actions Inactive

jamesbroadhead added 3 commits April 28, 2026 17:33

Merge remote-tracking branch 'origin/main' into fix-auth-login-error-…

395ca7d

…formatting # Conflicts: # libs/auth/error.go # libs/auth/error_test.go

jamesbroadhead temporarily deployed to test-trigger-is May 5, 2026 14:57 — with GitHub Actions Inactive

simonfaltum reviewed May 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve auth token error formatting for easier copy-paste#5115

Improve auth token error formatting for easier copy-paste#5115
jamesbroadhead wants to merge 9 commits intomainfrom
fix-auth-login-error-formatting

jamesbroadhead commented Apr 28, 2026

Uh oh!

github-actions Bot commented Apr 28, 2026 •

edited

Loading

Uh oh!

simonfaltum left a comment

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum left a comment

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum May 7, 2026

Uh oh!

simonfaltum May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		* `databricks api` now works against unified hosts. Adds `--account` to scope a call to the account API and `--workspace-id` to override the workspace routing identifier per call. A `?o=<workspace-id>` query parameter on the path (the SPOG URL convention used by the Databricks UI) is also recognized as a per-call workspace override, so URLs pasted from the browser route correctly.

		* Improve `auth token` error formatting for easier copy-paste of login commands ([#4602](https://github.com/databricks/cli/pull/4602))

Conversation

jamesbroadhead commented Apr 28, 2026

Changes

Why

Tests

Uh oh!

github-actions Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approval status: pending

/cmd/auth/ - needs approval

/cmd/root/ - needs approval

/libs/auth/ - needs approval

General files (require maintainer)

Uh oh!

simonfaltum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

simonfaltum left a comment

Choose a reason for hiding this comment

Review summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 28, 2026 •

edited

Loading

`/cmd/auth/` - needs approval

`/cmd/root/` - needs approval

`/libs/auth/` - needs approval